Skip to content

Conversation

@krystophny
Copy link
Collaborator

Summary

  • Fixed test_mpeg_consolidated test failure by properly registering animation save implementation
  • Added missing use fortplot_animation module import to enable save functionality
  • Changed from type-bound method calls to procedure calls to go through facade module registration

Root Cause Analysis

The test was failing because:

  1. It only imported fortplot module, not fortplot_animation
  2. It used type-bound method anim%save() which bypassed the facade module registration system
  3. The save implementation pointer remained unregistered, causing "Animation save implementation not initialized" errors

Technical Details

  • Animation save functionality requires the facade module fortplot_animation to register the implementation
  • Type-bound method anim%save() calls core module directly, skipping registration
  • Procedure call save_animation() goes through facade module which triggers registration via register_save_implementation()

Test Results

  • MPEG file now generated successfully (1619 bytes with valid MP4 structure)
  • Structure validation passes (detects MP4 box headers)
  • Error handling works correctly for invalid formats
  • No regressions in related tests (test_public_api, test_new_modules_integration)

Test Output

=== CONSOLIDATED MPEG VALIDATION TESTS ===
TEST: Basic MPEG Generation and Validation
✓ MPEG generation: PASS
✓ File existence: PASS
✓ File size: 1619 bytes
✓ Structure validation: T
TEST: Error Handling
✓ Invalid format rejection: PASS
=== All consolidated MPEG tests passed ===

Fixes #388

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

…tation

- Add missing fortplot_animation module import to register save implementation
- Change from type-bound anim%save() to procedure save_animation() calls
- Type-bound method bypassed facade module registration system
- Test now generates valid MPEG files (1619 bytes) with proper structure
- Error handling for invalid formats working correctly

Fixes #388
@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
test/test_mpeg_consolidated.f90 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@krystophny krystophny merged commit 7453393 into main Aug 26, 2025
4 of 5 checks passed
@krystophny krystophny deleted the fix-test-mpeg-consolidated-388 branch August 26, 2025 15:56
krystophny added a commit that referenced this pull request Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix: investigate test_mpeg_consolidated failure unrelated to ylabel rotation

2 participants